fix: prevent tests from writing to real ~/.spawn/history.json#2420
fix: prevent tests from writing to real ~/.spawn/history.json#2420
Conversation
Bun's os.homedir() reads from getpwuid() and ignores runtime changes to process.env.HOME. Named imports capture the native function binding, so patching os.homedir on the default export doesn't propagate. This caused all test files using homedir() to write .spawn-test-* dirs to the real home directory instead of the preload sandbox. Add getUserHome() helper to shared/ui.ts that prefers process.env.HOME, replace all direct homedir() calls in production and test code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test preload now sets SPAWN_HOME to the sandbox directory by default, so tests that call cmdRun/saveSpawnRecord without explicitly setting SPAWN_HOME no longer write to the real ~/.spawn/history.json. Add fs-sandbox.test.ts that verifies the sandbox is correctly configured (HOME, SPAWN_HOME, XDG vars all point to temp). Update testing.md with mandatory filesystem isolation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
louisgv
left a comment
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 5588a21
Findings
No security vulnerabilities found. This PR implements defensive security improvements by isolating test filesystem writes to temporary directories.
What this PR does:
- Adds test sandbox preload that redirects HOME/SPAWN_HOME/XDG_* to temp directories
- Prevents tests from accidentally writing to real user files (~/.spawn/history.json, ~/.ssh/, etc.)
- Introduces getUserHome() helper to work around Bun's os.homedir() ignoring process.env.HOME
- Adds fs-sandbox.test.ts guardrail to verify the sandbox is active
- Replaces all test imports of 'homedir' from node:os with process.env.HOME
Security impact: ✅ POSITIVE - prevents test pollution and protects developer environments
Tests
- bun test: ✅ PASS (1469 tests pass, 0 fail)
- bash -n: N/A (no shell scripts modified)
-- security/pr-reviewer
Merge Conflicts DetectedThis PR has been approved from a security perspective but cannot be merged due to conflicts with the main branch. Required action: Please rebase this PR on the latest git fetch origin
git rebase origin/main
git push --force-with-leaseOnce the conflicts are resolved, the PR can be merged immediately (all tests pass, security review complete). -- security/pr-reviewer |
|
Superseded by a new PR with merge conflicts resolved and rebased on current main. Also adds root-level bunfig.toml so |
2 participants
Summary
Builds on #2417. Tests that call
cmdRun/saveSpawnRecordwithout explicitly settingSPAWN_HOMEwere writing to the real~/.spawn/history.jsonon the developer's machine.SPAWN_HOMEto the sandbox inpreload.tsas the global default — no individual test can miss itfs-sandbox.test.tsguardrail that verifiesHOME,SPAWN_HOME, andXDG_CACHE_HOMEall point to the temp sandbox.claude/rules/testing.mdwith mandatory filesystem isolation rules (never importhomedirfromnode:os, always useprocess.env.HOME, etc.)3 test files were silently modifying
~/.spawn/history.json:commands-error-paths.test.tscommands-resolve-run.test.tscommands-swap-resolve.test.tsTest plan
bunx @biomejs/biome check src/— 0 errorsbun test— 1469 pass, 0 fail~/.spawn/history.jsonmtime unchanged before/after full test suitefs-sandbox.test.tspasses (5 assertions)🤖 Generated with Claude Code